fix(miner): claimNextBatch can claim the wrong host's row across two forge hosts by JSONbored · Pull Request #5594 · JSONbored/loopover

JSONbored · 2026-07-13T07:42:57Z

Summary

Fixes the blocker flagged in review on #5583: portfolio-queue-manager.js's caps-aware batch claiming encoded queue items via bare repoFullName+identifier, with no apiBaseUrl dimension. Since #5563 scoped the underlying store by (apiBaseUrl, repoFullName, identifier), two forge hosts can now legitimately share a repoFullName+identifier pair — and when they do, the engine's selection (which only ever sees the opaque id string) could select either host's item, but claimNextBatch's selectFn always defaulted target.apiBaseUrl to github.com, so batchClaim could mark a different row in_progress than the one the engine actually selected — a real data-integrity bug, not a safe no-op as I'd originally (incorrectly) documented.

queueItemId/parseQueueItemId now encode and decode apiBaseUrl as part of the id round-trip, so claimNextBatch always claims the exact row the engine selected, never a same-name row on the wrong host.
Also hardens the two already-shipped Scope local ledger keys by forge host, not bare repoFullName #5563 migrations (claim-ledger.js, portfolio-queue.js) with INSERT OR IGNORE for their table-rebuild copy step, matching run-state.js's and governor-state.js's (fix(miner): scope run-state by forge host, not bare repoFullName #5585, fix(miner): scope governor_reputation_history by forge host, not bare repoFullName #5591) fixes for the same class of bug: a legacy row with an already-invalid status/state value (this store's own read path already fails closed on those) would violate the rebuilt table's CHECK constraint and abort the whole migration, permanently breaking that file, instead of being dropped as the corrupt garbage it already was.

Advances #5563.

Test plan

New regression test proves claimNextBatch claims the correct host's row when two hosts share a repoFullName+identifier (mutation-tested: reverted the queueItemId fix, confirmed the test fails with the expected invalid_queue_item_id error, restored and reconfirmed green)
New regression tests for claim-ledger.js/portfolio-queue.js prove INSERT OR IGNORE drops a corrupted legacy row instead of crashing the migration
npx vitest run test/unit/miner-*.test.ts — 1645/1645 passing
npx tsc --noEmit clean
npm run build:miner clean
100% patch coverage on touched lines/branches (verified via lcov branch-diff against this PR's hunks, including the entriesToPortfolioQueue apiBaseUrl-fallback ternary's both branches)

…forge hosts portfolio-queue-manager.js's caps-aware batch claiming encoded queue items into ids via bare repoFullName+identifier, with no apiBaseUrl dimension. Since #5563 scoped the underlying store by (apiBaseUrl, repoFullName, identifier), two hosts can now legitimately share a repoFullName+identifier pair -- and when they do, the engine's selection (which only ever sees the opaque id string) could select either host's item, but claimNextBatch's selectFn always defaulted target.apiBaseUrl to github.com, so batchClaim could mark a DIFFERENT row in_progress than the one actually selected. queueItemId/parseQueueItemId now encode and decode apiBaseUrl as part of the id round-trip, so claimNextBatch always claims the exact row the engine selected, never a same-name row on the wrong host. Also hardens the two already-shipped #5563 migrations (claim-ledger.js, portfolio-queue.js) with INSERT OR IGNORE for their table-rebuild copy step, matching run-state.js's and governor-state.js's later fixes for the same class of bug: a legacy row with an already-invalid status/state value (this store's own read path already fails closed on those) would violate the rebuilt table's CHECK constraint and abort the whole migration, permanently breaking that file, instead of being dropped as the corrupt garbage it already was. Advances #5563.

superagent-security · 2026-07-13T07:44:11Z

Superagent didn't find any vulnerabilities or security issues in this PR.

cloudflare-workers-and-pages · 2026-07-13T07:44:14Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	loopover-ui	`719a140`	Commit Preview URL Branch Preview URL	Jul 13 2026, 07:44 AM

codecov · 2026-07-13T07:46:44Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.01%. Comparing base (b1d3e8d) to head (719a140).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5594   +/-   ##
=======================================
  Coverage   95.01%   95.01%           
=======================================
  Files         573      573           
  Lines       45534    45538    +4     
  Branches    14680    14680           
=======================================
+ Hits        43263    43268    +5     
+ Misses       1520     1519    -1     
  Partials      751      751

Flag	Coverage Δ
shard-1	`44.05% <100.00%> (-0.01%)`	⬇️
shard-2	`35.68% <0.00%> (+<0.01%)`	⬆️
shard-3	`31.94% <0.00%> (-0.23%)`	⬇️
shard-4	`31.76% <0.00%> (-1.08%)`	⬇️
shard-5	`32.94% <100.00%> (+1.09%)`	⬆️
shard-6	`44.40% <100.00%> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/gittensory-miner/lib/claim-ledger.js	`100.00% <ø> (ø)`
...es/gittensory-miner/lib/portfolio-queue-manager.js	`97.67% <100.00%> (+2.80%)`	⬆️
packages/gittensory-miner/lib/portfolio-queue.js	`96.90% <ø> (ø)`

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

loopover-orb · 2026-07-13T07:50:14Z

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-13 07:51:10 UTC}

7 files · 2 AI reviewers · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR correctly identifies and fixes a real data-integrity bug: queueItemId previously encoded only repoFullName+identifier, so with two forge hosts sharing that pair, batchClaim's selectFn defaulted apiBaseUrl to github.com and could claim the wrong host's row. The fix threads apiBaseUrl through queueItemId/parseQueueItemId and entriesToPortfolioQueue, verified by a genuine regression test that mutation-tests the fix (reverted and confirmed failure). The migration hardening (INSERT OR IGNORE for corrupted legacy rows) is a reasonable, narrowly-scoped defensive fix matching the pattern already used in run-state.js/governor-state.js, with tests proving the corrupt row is dropped rather than crashing the migration.

Nits — 5 non-blocking

The issue numbers (Scope local ledger keys by forge host, not bare repoFullName #5563, fix(miner): scope portfolio-queue by forge host, not bare repoFullName #5583) referenced in comments at portfolio-queue-manager.js:14-15 are fine as issue references, not magic numbers needing extraction into constants — the external brief's flag here is a false positive.
packages/gittensory-miner/lib/portfolio-queue-manager.js: consider validating that apiBaseUrl round-trips correctly if it ever contains the '::' separator itself, since queueItemId now joins three fields with the same separator and parseQueueItemId only splits on the first two occurrences.
Consider adding a test where apiBaseUrl itself contains '::' to confirm parseQueueItemId still resolves correctly (repoFullName/identifier are already documented as not containing '/' issues, but apiBaseUrl is a URL and could theoretically contain arbitrary chars).
The doc comment in portfolio-queue-manager.js:8-13 is thorough and clearly explains the historical bug — good practice for this kind of subtle wiring fix, worth keeping as the pattern for similar future changes.
nit: packages/gittensory-miner/lib/portfolio-queue-manager.js:17 changes the exported queueItemId runtime arity, so any plain-JS internal caller that still passes two arguments would produce a malformed host-scoped id; the TypeScript declaration helps, but a short compatibility assertion or call-site sweep note would make this safer.

Concerns raised — review before merging

No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

📋 Copy for AI agents — paste into your coding agent

Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.

Signal	Result	Evidence
Code review	❌ 2 blockers	2 reviewers, synthesized
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 44 registered-repo PR(s), 36 merged, 379 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 44 PR(s), 379 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.
Improvement	✅ Minor	risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — This directly fixes a real multi-host row-claiming integrity issue and adds targeted migration resilience for an already-shipped schema transition.

Review context

Author: JSONbored
Role context: owner (maintainer lane)
Public audience mode: oss maintainer
Lane context: Repository is configured for direct PR review.
Public profile languages: not available
Official Gittensor activity: 44 PR(s), 379 issue(s).
PR-specific overlap: none found.

Contributor next steps

Treat this as maintainer-lane context rather than normal contributor-lane activity.
Explain no-issue PR.
Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

Related work = same linked issue, overlapping active PRs, or title/path similarity.
Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

@gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
@gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

Re-run Gittensory review

…re repoFullName (#5595) deny_rule_proposals' PRIMARY KEY (repo_full_name, id) let two forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo share one proposal row (and its maintainer approval decision). Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name, id). This file has no schema-version framework of its own (unlike the package's other local stores) -- it uses a raw DatabaseSync connection, no applySchemaMigrations. Follows governor-state.js's idempotent column-presence-gated rebuild convention instead of introducing a new framework dependency here for the first time. Uses INSERT OR IGNORE for the copy step, matching the fix already applied to every other #5563 migration in this epic: a legacy row with an already-invalid status value (this store's own CHECK-constrained schema already rejects those) is dropped, not a migration-aborting crash. Threads an optional apiBaseUrl through refreshProposals/listProposals/ setProposalStatus/resolveEffectiveRules. initDenyHookSynthesisStore has no real callers yet (feeds the consumption surface #2343 will eventually wire into evaluateDenyHooks; this store owns derivation + audit, not live hook interception), so there is no CLI surface or call site to thread through -- purely the storage-layer fix, mirroring governor_reputation_history's scaffold-only precedent. Closes #5563 (5th and final store: claim-ledger.js in #5576, portfolio-queue.js in #5583, run-state.js in #5585, governor-state.js in #5591, plus the claimNextBatch/migration hardening follow-up in #5594).

github-actions Bot deployed to preview/pr-5594 July 13, 2026 07:44 View deployment

JSONbored mentioned this pull request Jul 13, 2026

fix(miner): scope deny-hook synthesis proposals by forge host, not bare repoFullName #5595

Merged

5 tasks

loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 13, 2026

loopover-orb Bot assigned JSONbored Jul 13, 2026

loopover-orb Bot added the manual-review Gittensor contributor context label Jul 13, 2026

JSONbored added this to Autonomous Miner System (Loopover AMS) Jul 13, 2026

github-project-automation Bot moved this to Todo in Autonomous Miner System (Loopover AMS) Jul 13, 2026

JSONbored added this to the Miner Wave 4 — AMS Hardening & Packaging milestone Jul 13, 2026

JSONbored merged commit 2cd2aa8 into main Jul 13, 2026
19 checks passed

JSONbored deleted the hardening-forge-scope-followups branch July 13, 2026 08:00

github-project-automation Bot moved this from Todo to Done in Autonomous Miner System (Loopover AMS) Jul 13, 2026

github-actions Bot mentioned this pull request Jul 14, 2026

chore(release): cut miner v1.0.0 #5712

Merged

Navigation Menu

Uh oh!

fix(miner): claimNextBatch can claim the wrong host's row across two forge hosts - #5594

fix(miner): claimNextBatch can claim the wrong host's row across two forge hosts#5594
JSONbored merged 1 commit into
mainfrom
hardening-forge-scope-followups

JSONbored commented Jul 13, 2026

Uh oh!

superagent-security Bot commented Jul 13, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Uh oh!

codecov Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

loopover-orb Bot commented Jul 13, 2026 •

edited

Loading

⏸️ Gittensory review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JSONbored commented Jul 13, 2026

Summary

Test plan

Uh oh!

superagent-security Bot commented Jul 13, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Deploying with Cloudflare Workers

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

loopover-orb Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏸️ Gittensory review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 13, 2026 •

edited

Loading

loopover-orb Bot commented Jul 13, 2026 •

edited

Loading